objects.py: fix group-algebra tensor products and qudit-operator parsing - #575
Merged
Conversation
….build ChainComplex.tensor_product fills structurally-zero blocks of each boundary operator with integer zeros. Over a group algebra the result was finalized with a bare array view plus a private _ring assignment, leaving those zeros as plain integers rather than ring members. Whenever a zero block survived -- i.e. for any product whose total complex has three or more links -- an operator mixed ring members with integers: it still satisfied d.d = 0, but lifting it to matrices (lift / regular_lift / to_field_array / transpose) crashed on the integer entries. Rebuild each operator through RingArray.build so every entry is coerced into a ring member. Add a tensor-product test over GF(3)[C3] with three links that validates the complex and lifts every boundary operator. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the jargon about zero blocks; state plainly that each operator is rebuilt so every entry is a ring member. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
QuditPauli.from_string promised a ValueError with an "Invalid qudit operator" message for bad input, but an empty factor (e.g. "*", "X(1)*", "") indexed an empty string and raised IndexError, and str.isnumeric() admitted unicode numerics such as the superscript two that int() then rejected with an unrelated message. Index the first character safely and validate digits with str.isdecimal(), so every malformed input raises the documented error. Also correct the swap_xz error message (it named a nonexistent Pauli.dual_xz) and a docstring typo in ChainComplex._validate_ops. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the unused ChainComplex._group annotation, the unused CayleyComplex._graph class attribute, the "L"/"R" edge labels in build_cayley_graph (read nowhere), and Node.__hash__ (the frozen dataclass generates an identical hash from its fields). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tag each Cayley-graph edge with type="L"/"R" to record whether it comes from the left- or right-acting generating subset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…happy ChainComplex.ops is typed as the union NDArray | RingArray, so calling .lift() on a loop variable fails mypy (ndarray has no .lift). Assert each operator is a RingArray before lifting -- which also sharpens the test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AI-assisted summary
Bug fix.
ChainComplex.tensor_productover a group algebra built boundary operators whose all-zero blocks were plain integers instead of ring elements. The result still passed the chain-complex check, but any complex with three or more links crashed when its operators were later lifted to matrices (lift,transpose, and so on). The code families don't reach this today -- they use the finite-field path -- but the method is public, so it is a real latent bug. Each operator is now rebuilt so every entry is a ring element, and a group-algebra test lifts every operator.Parsing.
QuditPauli.from_stringnow raises its documented "Invalid qudit operator" error for all malformed input. Previously an empty factor (such as "*" or "") raisedIndexError, and non-decimal digits raised a different, confusing error.Cleanup. Corrected the
swap_xzerror message (it named a method that does not exist), fixed a docstring typo, and removed dead code: an unused attribute on each ofChainComplexandCayleyComplex, unused edge labels in the Cayley graph, and a redundantNode.__hash__.